Repository navigation
test: Catch alert rules that break only inside vmalert - #555
Closed
andrewb3000 wants to merge 1 commit into
Closed
andrewb3000 wants to merge 1 commit into
andrewb3000 wants to merge 1 commit into
Conversation
andrewb3000
force-pushed
the
fix/513-ready-hosts-labelset
branch
from
October 27, 2026 12:00
f3b9353 to
823a05b
Compare
The ready compute host count shipped broken (#513): its fallback read back the group's own stored output, which carries vmalert's external label, and vmalert rejected every evaluation as two series with one labelset. The rule was checked with inline queries, which never add that label, so nothing caught it before production. Key features/changes: - Add a vmalert-tool unit test for the capacity recording rules. It covers cells with all, some and no hosts ready, a cordoned host, and a down host whose kubelet series has stopped - Add a load-only test for each other rule file, so a broken expression fails, and fail when any VMRule is named by no test, since vmalert-tool loads only the rule files a test names - Run the tests with an external label set, at the vmalert version the platform runs, so a label collision fails here as it fails in vmalert - Fail on an unreadable or multi-document manifest, a test file with no tests, or a yq other than mikefarah v4, rather than passing with nothing checked. VMRules saved as .yml are picked up too - Run the tests and the overlay builds in CI whenever the alert component changes vmalert-tool does not print a rule's evaluation error. It panics with a nil pointer dereference in the rule's exec, because it never registers the error counter that exec increments. The script spots that panic and prints a hint naming the usual cause. Against the rule from before #549 the test exits 2 on that panic; against main it passes.
andrewb3000
force-pushed
the
fix/513-ready-hosts-labelset
branch
from
October 27, 2026 12:11
823a05b to
e43b160
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The count of ready compute hosts per cell has failed every evaluation in the deployed environment since 21 October, so the infra alerts for a cell with no ready host, or with hosts down, cannot fire there.
It broke because its fallback reads back a stored series that carries the alert evaluator's environment label, and the inline queries used to check the rule never add that label.
This adds unit tests that load every alert rule file into vmalert-tool, at the version the platform runs, with that label set, and runs them in CI whenever the alert rules change.
The fix itself landed in #549 and already runs in a test environment, and deployed environment picks it up with the next compute release.
Limits
The capacity test checks the recorded host counts that ComputeCellNoHostReady and ComputeCellHostsUnavailable read, not those alerts, which live in the infra repo.
vmalert-tool does not print a rule's evaluation error. It panics with a nil pointer dereference instead, so the test run adds a hint naming the usual cause.
The failing run against the rule from before #549 was shown locally with the v1.121.0 release binary, not by a CI run. CI only exercises the passing path.
Test plan
Related to #513